Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add build.cssTarget option #5132

Merged
merged 3 commits into from
Oct 9, 2021
Merged

feat: add build.cssTarget option #5132

merged 3 commits into from
Oct 9, 2021
+56 −3

Conversation

haoqunjiang
Copy link
Member

@haoqunjiang haoqunjiang commented Sep 29, 2021 •
edited
Loading

Description

fixes #4746
fixes #5070
fixes #4930

Additional context

This option was discussed in previous meetings.
But I mistakenly thought #4658 had fixed the issue, so I didn't implement it until now.

The original PR made it possible to work around the issue in legacy browsers and projects without dynamic imports.

But in cases like #4746, if users target chrome61, import() will be transformed by esbuild to require(), leading to errors.

So we have to add this new option.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@patak-dev
Copy link
Member

Should we call this build.cssTarget in case there are other transforms apart from minification affected in the future?

@haoqunjiang
Copy link
Member Author

Yeah, makes sense.

@haoqunjiang haoqunjiang changed the title feat: add build.cssMinifyTarget option feat: add build.cssTarget option Sep 29, 2021
@Shinigami92 Shinigami92 added the p2-nice-to-have Not breaking anything but nice to have (priority) label Sep 29, 2021
@winterkind
Copy link

winterkind commented Oct 4, 2021 •
edited
Loading

Hi, may I ask what the label "on hold" means? No complaints, I am just the one who opened #4930 and trying to plan my next steps regarding it.

@patak-dev
Copy link
Member

Hi, may I ask what the label "on hold" means?

We want to discuss the naming of the option with the rest of the team before merging. This PR is probably going to be part of next week's release.

@patak-dev patak-dev removed the on hold label Oct 8, 2021
@IainZhuo
Copy link

IainZhuo commented Oct 9, 2021

Hi, may I ask what the label "on hold" means?

We want to discuss the naming of the option with the rest of the team before merging. This PR is probably going to be part of next week's release.

What is the final name after the discussion? I want to find a description of this configuration on the documentation.

@patak-dev patak-dev merged commit b17444f into vitejs:main Oct 9, 2021
@patak-dev
Copy link
Member

It will be build.cssTarget

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shinigami92 Shinigami92 Shinigami92 approved these changes

@patak-dev patak-dev patak-dev approved these changes

Assignees
No one assigned
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Milestone
No milestone
5 participants
@haoqunjiang @patak-dev @winterkind @IainZhuo @Shinigami92